fix(mcp): launch the Chrome profile that has the extension installed#41939
Conversation
| args.push(`--user-data-dir=${testUserDataDir}`); | ||
| const userDataDir = testUserDataDir ?? this._userDataDir; | ||
| const profileDirectory = userDataDir ? await findPlaywrightExtensionProfile(userDataDir) : undefined; | ||
| if (profileDirectory) |
| const userDataDir = test.info().outputPath('multi-profile'); | ||
| await fs.mkdir(path.join(userDataDir, 'Profile 5', 'Extensions', extensionId), { recursive: true }); | ||
|
|
||
| const executablePath = test.info().outputPath('echo.sh'); |
There was a problem hiding this comment.
can we do e2e test instead and run chrome with 2 real profiles?
| const userDataDir = test.info().outputPath('multi-profile-last-used'); | ||
| await fs.mkdir(path.join(userDataDir, 'Default', 'Extensions', extensionId), { recursive: true }); | ||
| await fs.mkdir(path.join(userDataDir, 'Profile 5', 'Extensions', extensionId), { recursive: true }); | ||
| await fs.writeFile(path.join(userDataDir, 'Local State'), JSON.stringify({ profile: { last_used: 'Profile 5' } })); |
There was a problem hiding this comment.
Ditto. We already parse chromium private data structures and we want to know if it breaks.
| const userDataDir = testUserDataDir ?? this._userDataDir; | ||
| const profileDirectory = userDataDir ? await findPlaywrightExtensionProfile(userDataDir) : undefined; | ||
| if (profileDirectory) | ||
| args.push(`--profile-directory=${profileDirectory}`); |
There was a problem hiding this comment.
maybe we should accept an option for explicit profile selection (the directory names don't match profile names in the ui though)?
There was a problem hiding this comment.
added a --profile-directory option and PLAYWRIGHT_MCP_PROFILE_DIRECTORY environment variable
There was a problem hiding this comment.
Commented below, let's remove it for now.
This comment has been minimized.
This comment has been minimized.
yury-s
left a comment
There was a problem hiding this comment.
let's drop --profile-directory option for now, otherwise looks good
| .option('--output-dir <path>', 'path to the directory for output files.') | ||
| .option('--output-max-size <bytes>', 'Threshold for evicting old output files, in bytes.', numberParser) | ||
| .option('--port <port>', 'port to listen on for SSE transport.') | ||
| .option('--profile-directory <directory>', 'in extension mode, the browser profile directory to connect to (for example "Default" or "Profile 1"). Defaults to the profile that has the extension installed.') |
There was a problem hiding this comment.
Let's hold off, we already have --user-data-dir option and this just adds to the confusion (there is no easy way for the user to guess which of "Profile N" corresponds to the user profile "Foo"). If there is demand we can add it later. We'd also need to expose it as an option in playwright-cli.
| const userDataDir = testUserDataDir ?? this._userDataDir; | ||
| const profileDirectory = userDataDir ? await findPlaywrightExtensionProfile(userDataDir) : undefined; | ||
| if (profileDirectory) | ||
| args.push(`--profile-directory=${profileDirectory}`); |
There was a problem hiding this comment.
Commented below, let's remove it for now.
| headless: false, | ||
| ignoreDefaultArgs: ['--enable-automation'], | ||
| args: [ | ||
| `--profile-directory=${profile}`, |
There was a problem hiding this comment.
does it actually create a new profile?
There was a problem hiding this comment.
i believe so, but given your comment below im removing this
| await createProfile(userDataDir, 'Default', mcpBrowser); | ||
| await createProfile(userDataDir, 'Profile 1', mcpBrowser, pathToExtension); | ||
|
|
||
| const executablePath = testInfo.outputPath('echo.sh'); |
There was a problem hiding this comment.
this is still just a unit test for the logic in our code that constructs the parameters, in that case let's just keep one simple test and it's okay to create mock profiles manually
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
extension mode validated that the playwright extension was installed in any of Chrome's profiles under the user data directory, but then launched without `--profile-directory`, meaning it could open a different profile than the one that was checked when the opened profile lacks the extension, `connect.html` can't load and the connection silently hangs for ~120s resolve the profile that actually has the extension (preferring the last-used profile from Local State so we don't switch away from the user's session unnecessarily) and pass it as `--profile-directory`
Test results for "MCP"1 failed 7785 passed, 1262 skipped Merge workflow run. |
🟡 One failure I can't pin down — leaning unrelatedHi, I'm the Playwright bot and I took a first look at the CI failures. The latest run has a single failure: DetailsOverall: this PR only touches MCP extension mode. All four changed paths — Uncertain
Triaged by the Playwright bot - agent run |
extension mode validated that the playwright extension was installed in any of Chrome's profiles under the user data directory, but then launched without
--profile-directory, meaning it could open a different profile than the one that was checkedwhen the opened profile lacks the extension,
connect.htmlcan't load and the connection silently hangs for ~120sresolve the profile that actually has the extension (preferring the last-used profile from Local State so we don't switch away from the user's session unnecessarily) and pass it as
--profile-directoryfixes #41916